Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cygwin target. #134999

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

Berrysoft
Copy link
Contributor

@Berrysoft Berrysoft commented Jan 1, 2025

This PR simply adds cygwin target together with msys2 target, based on @ookiineko 's (the account has been deleted) work on cygwin target. My full work is here: master...Berrysoft:rust:dev/cygwin

I have succeeded in building a new rustc for cygwin target, and eventually distributed a new version of fish-shell (rewritten by Rust) for MSYS2.

I will open a new PR to fix std if this PR is accepted.

@rustbot
Copy link
Collaborator

rustbot commented Jan 1, 2025

Failed to set assignee to compiler-team: cannot assign: response: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/assignees#add-assignees-to-an-issue","status":"404"}

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 1, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 1, 2025

These commits modify compiler targets.
(See the Target Tier Policy.)

Some changes occurred in src/doc/rustc/src/platform-support

cc @Noratrieb

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Noratrieb
Copy link
Member

r? compiler

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@workingjubilee
Copy link
Member

This should be x86_64-pc-windows-cygwin, if anything.

@mati865
Copy link
Contributor

mati865 commented Jan 1, 2025

I'm not sure if two separate triples are necessary given msys2/MSYS2-packages#3012, but I think there is no solution for having to distinguish between Cygwin and MSYS2 DLL yet.

@Berrysoft
Copy link
Contributor Author

@workingjubilee

This should be x86_64-pc-windows-cygwin, if anything.

@jieyouxu

Question: why does this need to be empty, that seems wrong?

I will answer these two questions here. Ookiineko chose x86_64-pc-cygwin instead of x86_64-pc-windows-cygwin, and I agree with this choice. The target_os is cygwin but not windows, and msys2 is only an variant (or, target_env, so the original cygwin target has an empty target_env). Here are some advantages of this choice:

  • Cygwin is a POSIX-compatible layer on Windows. From an engineering view, it would be more convenient to assume it as "unix" but not "windows". A typical cygwin program uses file descriptor instead of file handle, uses malloc instead of HeapAlloc, and uses fork. Cygwin is not a different native ABI just like MinGW and MSVC, but much like a subsystem.
  • Make the maintainer (me) easier. Most code in the std assumes that "windows" is not "unix", and "unix" has it's common ways to do many affairs. For example, RawFd is defined for unix, and RawHandle is defined for windows. If cygwin is defined as an independent unix os, I don't have to change and maintain a lot cfgs to make it work.
  • To make others' porting easier. If "cygwin" is added as a new target_os in unix family, most crates only need small changes or even no changes to port to this new target. However, if "cygwin" is added as a new ABI on "windows", the whole environment needs a relative massive change to port to Cygwin. All code with cfg(unix) might need to be changed to cfg(any(unix, target_env = "cygwin")) or even cfg(any(unix, all(windows, target_env = "cygwin"))), and all code with cfg(windows) need to be changed to cfg(all(windows, not(target_env = "cygwin"))). That's too ugly.

Therefore, I would suggest that just treat Cygwin as an Unix OS.

@Berrysoft
Copy link
Contributor Author

I'm not sure if two separate triples are necessary given msys2/MSYS2-packages#3012, but I think there is no solution for having to distinguish between Cygwin and MSYS2 DLL yet.

Well... To be honest MSYS2 is just a fork of Cygwin, with some user-friendly changes. From the compiler's view, the differences are only

  • Linker name
  • Runtime lib name

However, the cygwin dll and msys2 dll should not be mixed. They all maintain their own services and layers. It might cause some misc errors when a cygwin exe linked to a msys2 dll trying to call fork.

I add the -msys2 target for convenience. If it's not added, the users of MSYS2 might need to tweak their environment a little bit - symlinking the linkers and libs, maybe.

P.S. I test the cygwin target on MSYS2, actually. I just symlinked the linker and libcygwin.a.

@jieyouxu
Copy link
Member

jieyouxu commented Jan 2, 2025

@Berrysoft could you file an MCP for adding these two targets? Usually we wouldn't ask for MCPs for adding Tier 3 targets, but in this case I think it's warranted to allow compiler team members to discuss how to sort out the target_os vs target_env situation for cygwin and msys2 targets (in addition to what to use for the target tuple in lieu of other windows-ish targets). Because it feels to me that with this "treat it as Unix OS" scheme, we might have some issues with "Unix-like but not Unix enough" and "Windows-like but not Windows enough" simultaneously.

For instance, say if I have something that is conditioned on cfg(target_os = "windows") or cfg(not(target_os = "windows")), will the cygwin or msys2 target qualify or not?

@Berrysoft
Copy link
Contributor Author

Berrysoft commented Jan 2, 2025

we might have some issues with "Unix-like but not Unix enough" and "Windows-like but not Windows enough" simultaneously.

Well, I would say that Cygwin is Unix enough in most times, and also Windows enough all the time (because it is really running on Windows). However, from the development view, I don't think target_os = "windows" should include Cygwin target. cfg(target_os = "windows") usually means

  • There are file handles and socket handles, not file descriptors. However, Cygwin uses file descriptors just like other Unix.
  • There is CreateProcess instead of fork + exec or posix_spawn. However, Cygwin provides all POSIX methods.
  • The filesystem is Windows-like (backslash, drive letters, UNC paths, etc.). However, Cygwin uses Unix-like paths. It even provides /dev and /proc.

Yes, treating Cygwin as Unix has some drawbacks:

  • fork is very slow on Cygwin. The unix shells work on Cygwin, but usually slower than WSL1 & WSL2.
  • IO is not that fast, compared to native Win32 methods. Cygwin doesn't provide kqueue or epoll, while Windows provides IOCP for high-performance IO. I would say that even basic filesystem IO is not that fast either, but it's fine. Everyone using cygwin or MSYS2 knows that.
  • I'm not even sure if X11 still runs well in Cygwin. Some cygwin executables (e.g., mintty) calls Win32 APIs directly to create GUI. If we treat cygwin as unix, the windows-rs crate might need to think how to port for cygwin target... but anyway they need to write some code for a new windows target, isn't it?

Therefore I would say that treating cygwin as unix is better. Cygwin runs on Windows, but it's better for us to not know it's Windows. If we add a new windows-cygwin target, and try to use native Win32 API instead of the POSIX layer provided by Cygwin, then the thing is not that meaningful. I would like "unix code" running on Windows. If a unix program, e.g., fish-shell, could not be compiled to Windows with this target, it would be meaningless.

@jieyouxu Do you still think that I need to file an MCP? It's not hard for me to just opening an issue, but I'm afraid of a large debate, only focusing on whether cygwin is Unix or Windows.

Philosophers have only interpreted the world, in various ways; the point, however, is to change it.

I am going to change it.

@jieyouxu
Copy link
Member

jieyouxu commented Jan 2, 2025

Do you still think that I need to file an MCP? It's not hard for me to just opening an issue, but I'm afraid of a large debate, only focusing on whether cygwin is Unix or Windows.

That's exactly why I am requesting an MCP when usually Tier 3 targets don't need to, because this aspect is controversial. I intend to give the MCP some time, but if we fail to reach consensus, I'll re-nominate the MCP for compiler triage meeting to decide how to proceed.

I am going to change it.

That's completely fair. I'd like other compiler team members to vibe-check how this change is made.

@workingjubilee
Copy link
Member

hmm. I mean, I can run Windows programs on Linux, but

  • that doesn't make my Linux computer have target_os = "windows"
  • they don't really need modifications (usually), they either work or don't

@jieyouxu jieyouxu added the needs-mcp This change is large enough that it needs a major change proposal before starting work. label Jan 2, 2025
@Berrysoft
Copy link
Contributor Author

MCP opened: rust-lang/compiler-team#826

Zulip: https://rust-lang.zulipchat.com/#narrow/stream/233931-xxx/topic/Add.20new.20targets.20for.20Cygwin.20.28and.20MSYS2.29.20compiler-team.23826

@Noratrieb Noratrieb added S-waiting-on-MCP Status: PR has a compiler MCP and is waiting for the compiler MCP to complete. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 2, 2025
@jieyouxu jieyouxu removed the needs-mcp This change is large enough that it needs a major change proposal before starting work. label Jan 2, 2025
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 8, 2025
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Feb 8, 2025
…=chenyukang

Add cygwin target.

r? compiler-team

This PR simply adds cygwin target together with msys2 target, based on `@ookiineko` 's (the account has been deleted) [work](https://github.com/ookiineko-cygport/rust) on cygwin target. My full work is here: rust-lang/rust@master...Berrysoft:rust:dev/cygwin

I have succeeded in building a new rustc for cygwin target, and eventually distributed a new version of [fish-shell](https://github.com/Berrysoft/fish-shell/releases) (rewritten by Rust) for MSYS2.

I will open a new PR to fix std if this PR is accepted.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 8, 2025
…kingjubilee

Rollup of 13 pull requests

Successful merges:

 - rust-lang#134999 (Add cygwin target.)
 - rust-lang#135439 (Make `-O` mean `OptLevel::Aggressive`)
 - rust-lang#136397 (Add a comment pointing to ICE-136223)
 - rust-lang#136681 (resolve `llvm-config` path properly on cross builds)
 - rust-lang#136686 (Clean up `HashMap` and `HashSet` docs.)
 - rust-lang#136694 (Update minifier version to `0.3.4`)
 - rust-lang#136706 (compiler: mostly-finish `rustc_abi` updates)
 - rust-lang#136710 (Document `Sum::sum` returns additive identities for `[]`)
 - rust-lang#136724 (Make `AsyncFnOnce`, `AsyncFnMut`, `AsyncFn` non-`#[fundamental]`)
 - rust-lang#136727 (Have a break from review rotation)
 - rust-lang#136730 (transmutability: fix ICE when passing wrong ADT to ASSUME)
 - rust-lang#136736 (Small resolve refactor)
 - rust-lang#136746 (Emit an error if `-Zdwarf-version=1` is requested)

r? `@ghost`
`@rustbot` modify labels: rollup
Urgau added a commit to Urgau/rust that referenced this pull request Feb 9, 2025
…=chenyukang

Add cygwin target.

r? compiler-team

This PR simply adds cygwin target together with msys2 target, based on ``@ookiineko`` 's (the account has been deleted) [work](https://github.com/ookiineko-cygport/rust) on cygwin target. My full work is here: rust-lang/rust@master...Berrysoft:rust:dev/cygwin

I have succeeded in building a new rustc for cygwin target, and eventually distributed a new version of [fish-shell](https://github.com/Berrysoft/fish-shell/releases) (rewritten by Rust) for MSYS2.

I will open a new PR to fix std if this PR is accepted.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2025
Rollup of 5 pull requests

Successful merges:

 - rust-lang#134999 (Add cygwin target.)
 - rust-lang#135488 (Stabilize `vec_pop_if`)
 - rust-lang#136068 (crashes: more tests)
 - rust-lang#136694 (Update minifier version to `0.3.4`)
 - rust-lang#136760 (Fix unwrap error in overflowing int literal)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r-
needs reformatting

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 9, 2025
@bors
Copy link
Contributor

bors commented Feb 10, 2025

☔ The latest upstream changes (presumably #134740) made this pull request unmergeable. Please resolve the merge conflicts.

Berrysoft and others added 3 commits February 10, 2025 17:13
Co-authored-by: Ookiineko <[email protected]>
Co-authored-by: nora <[email protected]>
Co-authored-by: Jubilee <[email protected]>
@Berrysoft Berrysoft force-pushed the dev/new-cygwin-target branch from 402a52e to 8e5207e Compare February 10, 2025 09:56
@Berrysoft
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 10, 2025
@workingjubilee workingjubilee self-assigned this Feb 10, 2025
@workingjubilee
Copy link
Member

Cool.

@bors r=chenyukang,workingjubilee

@bors
Copy link
Contributor

bors commented Feb 10, 2025

📌 Commit 8e5207e has been approved by chenyukang,workingjubilee

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 10, 2025
@workingjubilee workingjubilee added the O-windows-gnu Toolchain: GNU, Operating system: Windows label Feb 10, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Feb 11, 2025
…=chenyukang,workingjubilee

Add cygwin target.

This PR simply adds cygwin target together with msys2 target, based on `@ookiineko` 's (the account has been deleted) [work](https://github.com/ookiineko-cygport/rust) on cygwin target. My full work is here: rust-lang/rust@master...Berrysoft:rust:dev/cygwin

I have succeeded in building a new rustc for cygwin target, and eventually distributed a new version of [fish-shell](https://github.com/Berrysoft/fish-shell/releases) (rewritten by Rust) for MSYS2.

I will open a new PR to fix std if this PR is accepted.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 11, 2025
Rollup of 6 pull requests

Successful merges:

 - rust-lang#134999 (Add cygwin target.)
 - rust-lang#135677 (Small `rustc_resolve` cleanups)
 - rust-lang#136699 (std: replace the `FromInner` implementation for addresses with private conversion functions)
 - rust-lang#136758 (tests: `-Copt-level=3` instead of `-O` in assembly tests)
 - rust-lang#136761 (tests: `-Copt-level=3` instead of `-O` in codegen tests)
 - rust-lang#136833 (compiler: die immediately instead of handling unknown target codegen)

Failed merges:

 - rust-lang#136808 (Try to recover from path sep error in type parsing)

r? `@ghost`
`@rustbot` modify labels: rollup
fmease added a commit to fmease/rust that referenced this pull request Feb 11, 2025
…=chenyukang,workingjubilee

Add cygwin target.

This PR simply adds cygwin target together with msys2 target, based on ``@ookiineko`` 's (the account has been deleted) [work](https://github.com/ookiineko-cygport/rust) on cygwin target. My full work is here: rust-lang/rust@master...Berrysoft:rust:dev/cygwin

I have succeeded in building a new rustc for cygwin target, and eventually distributed a new version of [fish-shell](https://github.com/Berrysoft/fish-shell/releases) (rewritten by Rust) for MSYS2.

I will open a new PR to fix std if this PR is accepted.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 11, 2025
Rollup of 7 pull requests

Successful merges:

 - rust-lang#134090 (Stabilize target_feature_11)
 - rust-lang#134999 (Add cygwin target.)
 - rust-lang#135025 (Cast allocas to default address space)
 - rust-lang#135408 (x86: make SSE2 required for i686 hardfloat targets and use it to pass SIMD types)
 - rust-lang#135549 (Document some safety constraints and use more safe wrappers)
 - rust-lang#136193 (Implement pattern type ffi checks)
 - rust-lang#136699 (std: replace the `FromInner` implementation for addresses with private conversion functions)

Failed merges:

 - rust-lang#136758 (tests: `-Copt-level=3` instead of `-O` in assembly tests)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows-gnu Toolchain: GNU, Operating system: Windows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.